refactor(persona): consolidate trajectories + ai-memory under memory (opt-in)#210
Conversation
… opt-in
Replace the top-level `recordTrajectories` boolean (default-on, enforced) with
two opt-in facets on the existing `memory` object. Both default OFF.
memory: {
enabled?, scopes?, ttlDays?, autoPromote?, dedupMs?, // long-form (unchanged)
trajectories?: boolean | { enabled?, autoCompact? }, // record the WHY
aiMemory?: boolean | { enabled?, dbPath? } // load ai-hist MCP (WHY+HOW recall)
}
Semantics:
- `memory` omitted → everything off. `memory: true` → long-form only (facets stay off).
- `memory.trajectories` gates runtime decision-trajectory recording.
- `memory.aiMemory` gates injecting the ai-hist MCP (npx -y -p ai-hist ai-hist-mcp).
Changes:
- persona-kit: PersonaMemoryConfig + PersonaTrajectoryConfig + PersonaAiMemoryConfig;
parseMemory parses both facets; new resolveTrajectoryRecording / resolveAiMemory
helpers; PersonaSpec drops recordTrajectories; PersonaSelection carries `memory`;
regenerated schema. interactive-spec injection unchanged (still gated on input.aiHist).
- cli + plan: gate ai-hist injection on resolveAiMemory(memory).enabled; dbPath override.
- workload-router: carry `memory` onto the selection.
- runtime: ctx gates the recorder on resolveTrajectoryRecording(memory); cloud-defaults
gates ai-hist MCP on resolveAiMemory(memory). Recorder's internal option unchanged.
- deploy: drop the recordTrajectories preflight enforcement (opt-in ⇒ nothing to enforce).
Reverses this session's earlier enforced-default-on decision per the new opt-in requirement.
Tests: persona-kit 268, deploy 177; cli + workload-router build. Runtime build to be
confirmed in claude-1's env (local agent-trajectories install is version-shimmed).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR refactors persona memory configuration from a flat ChangesPersona Memory Configuration Refactor
🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- proactive-issue-resolver: boolean form (trajectories: true, aiMemory: true)
- review-agent: object form (trajectories: { autoCompact }, aiMemory: { enabled })
Both compose with the existing long-form memory block. Parse-verified.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request replaces the top-level recordTrajectories boolean property in the persona specification with more granular, opt-in memory facets under memory (memory.trajectories and memory.aiMemory), which are now off by default. The CLI, deploy, runtime, and workload-router packages have been updated to parse, resolve, and utilize these new configurations. Feedback was provided to update the return type of resolveAiHistFromEnv in packages/runtime/src/cloud-defaults.ts to AiHistMcpConfig since it can no longer return undefined.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function resolveAiHistFromEnv( | ||
| env: NodeJS.ProcessEnv, | ||
| defaultTrajectoryRoot?: string | ||
| defaultTrajectoryRoot?: string, | ||
| dbPathOverride?: string | ||
| ): AiHistMcpConfig | undefined { |
There was a problem hiding this comment.
The function resolveAiHistFromEnv always returns an object of type AiHistMcpConfig (even if empty) and can never return undefined now that the WORKFORCE_AIHIST_DISABLED check has been removed. Updating the return type to AiHistMcpConfig makes the type signature more accurate and avoids unnecessary undefined checks by callers.
| function resolveAiHistFromEnv( | |
| env: NodeJS.ProcessEnv, | |
| defaultTrajectoryRoot?: string | |
| defaultTrajectoryRoot?: string, | |
| dbPathOverride?: string | |
| ): AiHistMcpConfig | undefined { | |
| function resolveAiHistFromEnv( | |
| env: NodeJS.ProcessEnv, | |
| defaultTrajectoryRoot?: string, | |
| dbPathOverride?: string | |
| ): AiHistMcpConfig { |
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="examples/review-agent/persona.json">
<violation number="1" location="examples/review-agent/persona.json:19">
P2: Enabling `trajectories` and `aiMemory` under `memory` contradicts the README's explicit warning that memory is not wired (`ctx.memory` is a stub in v1). This creates a misleading configuration where the config signals these features are active, but the underlying runtime infrastructure for this persona does not support them.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| "workspace" | ||
| ] | ||
| ], | ||
| "trajectories": { |
There was a problem hiding this comment.
P2: Enabling trajectories and aiMemory under memory contradicts the README's explicit warning that memory is not wired (ctx.memory is a stub in v1). This creates a misleading configuration where the config signals these features are active, but the underlying runtime infrastructure for this persona does not support them.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At examples/review-agent/persona.json, line 19:
<comment>Enabling `trajectories` and `aiMemory` under `memory` contradicts the README's explicit warning that memory is not wired (`ctx.memory` is a stub in v1). This creates a misleading configuration where the config signals these features are active, but the underlying runtime infrastructure for this persona does not support them.</comment>
<file context>
@@ -15,7 +15,14 @@
"workspace"
- ]
+ ],
+ "trajectories": {
+ "enabled": true,
+ "autoCompact": true
</file context>
The b986dba examples edit added memory.trajectories/aiMemory to review-agent; parse.test 'accepts the Relayfile-VFS example personas' deep-equals that memory and needed the expectation updated. (Missed because persona-kit tests were run before the examples commit.) persona-kit 268 pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
Fixed the validated issue in the PR: Added focused coverage:
Validation run:
|
|
✅ pr-reviewer applied fixes — committed and pushed Fixed the validated issue in the PR: Added focused coverage:
Validation run:
|
|
Fixed the validated PR breakage in parse.test.ts: the example persona assertion now expects the new Verified with: |
|
✅ pr-reviewer applied fixes — committed and pushed Fixed the validated PR breakage in parse.test.ts: the example persona assertion now expects the new Verified with: |
|
Reviewed PR #210 against the current checkout. I did not find any validated breakage that required code edits, so I left the source unchanged. Local verification passed:
|
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. Reviewed PR #210 against the current checkout. I did not find any validated breakage that required code edits, so I left the source unchanged. Local verification passed:
|
|
Fixed a parser bug in parse.ts: Verification: |
|
Fixed a parser bug in parse.ts: Verification: |
|
Reviewed PR #210 and made one small test-only fix: added coverage in parse.test.ts for the new Local validation passed: Note: the environment did not have a |
|
Reviewed PR #210 and made one small test-only fix: added coverage in parse.test.ts for the new Local validation passed: Note: the environment did not have a |
|
ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed. Reviewed PR #210 against the current checkout. I didn’t find any current breakage or stale bot-review fixes to apply, so I left the working tree source unchanged. Validated locally: That completed successfully: lint, typecheck including examples, and all package tests passed. |
What
Per Khaliq: fold trajectory recording and ai-memory recall into the existing
memoryobject as opt-in facets, replacing the top-levelrecordTrajectoriesboolean (which was default-on + enforced).New shape
true{ trajectories: true }{ aiMemory: true }memory.trajectories→ runtime records decision trajectories to<root>/<personaId>/compacted/*.json.memory.aiMemory→ persona loads the ai-hist MCP (npx -y -p ai-hist ai-hist-mcp) to recall the why+how. OptionaldbPathoverrides the history DB.Changes
PersonaMemoryConfiggainstrajectories/aiMemory; newPersonaTrajectoryConfig+PersonaAiMemoryConfig;parseMemoryparses both; new exportedresolveTrajectoryRecording/resolveAiMemoryhelpers;PersonaSpecdropsrecordTrajectories;PersonaSelectioncarriesmemory; schema regenerated. (Injection logic ininteractive-specunchanged — still gated oninput.aiHist.)resolveAiMemory(memory).enabled; honordbPath.memoryonto the selection.ctxgates the recorder onresolveTrajectoryRecording(memory);cloud-defaultsgates the MCP onresolveAiMemory(memory). Recorder's internal option name kept.recordTrajectoriespreflight enforcement (opt-in ⇒ nothing to enforce).This reverses the earlier enforced-default-on decision per the new opt-in requirement.
Tests / build
agent-trajectoriesinstall is version-shimmed (.ignored_quirk), so I couldn't fully build runtime here — the only tsc error was a pre-existingworkflowIdline in untouched code from the shimmed 0.5.6 version; my ctx/cloud-defaults edits produced zero type errors. @claude-1 to confirm the runtime build (82/82) in their env before merge — it's a coupled type change (removingrecordTrajectoriesfromPersonaSpec).🤖 Generated with Claude Code